Skip to content

Added implementation of filter refraction as a photon operator#103

Open
FedericoBerlfein wants to merge 8 commits into
mainfrom
filter_refraction_photop
Open

Added implementation of filter refraction as a photon operator#103
FedericoBerlfein wants to merge 8 commits into
mainfrom
filter_refraction_photop

Conversation

@FedericoBerlfein

@FedericoBerlfein FedericoBerlfein commented May 28, 2026

Copy link
Copy Markdown
Collaborator

Implementation of refraction from the WFI filters as a photon operator. The photon operator lives in the file filter_refraction.py and contains the relevant functions used in the photon operator. Modifications to __init__.py are simply to add the new file and functions within it. Modifications to config/default.yaml are to add refraction prior to charge diffusion. I believe this is the correct order, as this effect occurs before photons reach the detector.

A small breakdown of the functions in filter_refraction.py:

  • class RomanFilterRefraction: This class contains the GalSim photon operator. To initialize the operator, you need to specify the bandpass, SCA and SCA position, pixel scale, and callable function to get the index of refraction as a function of wavelength. The applyTo gets the lateral chromatic shifts induced by filter refraction and applies it directly to the photons based on their wavelength. The default for the pixel scale is the native roman scale (0.11 arcsec/pixel), and the default index of refraction comes from the Roman WFI filter substrate (Suprasil 3001)
  • class RomanFilterRefractionBuilder: This class builds the photon operator from the information in base, which includes the bandpass, SCA and SCA position.
  • _get_lateral_shifts: This is the main function called within applyTo that calculates the lateral chromatic shifts. This function calls other helper functions defined throughout the file.
  • Constants: At the top of the file, several relevant constants and coefficients are defined. Primarily, the manufacturer fits to the index of refraction and filter/telescope geometry (radius of curvature, distance between optical elements, filter thickness). In addition, I use a dictionary containing the SCA centers in FPA coordinates. This provides a fast and accurate way to estimate the FPA position at any SCA and SCA position without the need of external libraries (e.g. pysiaf)

@FedericoBerlfein FedericoBerlfein self-assigned this May 28, 2026
@FedericoBerlfein FedericoBerlfein linked an issue May 28, 2026 that may be closed by this pull request

@sidneymau sidneymau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few comments. The math throughout looks reasonable, but I'd have to dedicate a bit more time to fully review everything here



# ===========================================================================
# Roman WFI instrument constants

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a global reference these can come from? Not sure if this info exists already in romanisim or somewhere else? (cc @yuedong0607)

@FedericoBerlfein FedericoBerlfein Jun 5, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For most of them I can point to this documentation for reference. For the pupil demagnification I don't think there is a public reference. The pupil is not perfectly circular, and the demagnification can vary across the FOV. What I did initially was simply calculate the average across the FOV along the x-direction and use that. But to be more exact here, I updated the code to use the slightly more exact version, which uses a different demagnification for the x- and y-axis. I added some documentation in the constants that hopefully makes things clear.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for clarifying -- and I agree that it's good to use the slightly more exact version, especially if anyone in the future assumes that it has been implemented and is doing a study sensitive to the difference.

My original comment had more to do with, for example, the pixel size, scale, focal ratio, diameter, etc. -- I would imagine that there is some common reference for all of these values elsewhere in galsim/roman_imsim/romanisim, but I'm not sure where the base truth should live. If you think it's better to define these all locally and not use something else, that's fine, but I do think it's worth consideration

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it that makes sense. For the pixel size, scale, and diameter, I considered importing them from galsim.roman, but it felt odd to have 3 constants exported from there and everything else hard-coded, so I opted to keep everything defined locally. I could not find these constants defined in roman_imsim because I think the code just calls galsim.roman directly when it needs them.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuedong0607 probably knows best where these constants are defined most generally?

Comment thread roman_imsim/filter_refraction.py Outdated
Comment thread roman_imsim/filter_refraction.py Outdated
Comment thread roman_imsim/filter_refraction.py Outdated
Comment thread roman_imsim/filter_refraction.py Outdated
@FedericoBerlfein

Copy link
Copy Markdown
Collaborator Author

@sidneymau thank you for the comments! I think I addressed all of them above. I did a small update to the pupil demagnification, mostly to upgrade to a slightly more accurate version. Before I was using a single factor (for both x- and y-axis), but technically the demagnification can can be different in the x/y direction because the pupil is not perfectly circular. This affects the angle of incidence in each direction, but the change is truly quite minimal and does not really affect the effect in practice. I just thought to include it for completeness.

@sidneymau sidneymau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing to my earlier comments! Before approving, I think there are a few items to do:

  1. make sure our reference image action still passes and/or update appropriately (cc @arunkannawadi for thoughts on if this refraction should be part of the default configuration here)
  2. we should probably write some tests for all of the functions throughout

@FedericoBerlfein

Copy link
Copy Markdown
Collaborator Author

Regarding the tests, I can get started on that. I see that in the tests folder there are two files that do some broad-level tests rather than specific functions. How would we want to structure this? Add a new file specifically for photon operator tests?

@sidneymau

Copy link
Copy Markdown
Contributor

Yeah, the lack of unit tests is a big TODO for this project. I think a new file specifically for the photon operator -- or even for the filter refraction -- tests would be a good first step towards this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add filter-substrate refraction photon operator

2 participants